feat(feedback): Add screenshot attachment button to user feedback widget - #5828
feat(feedback): Add screenshot attachment button to user feedback widget#5828markushi wants to merge 18 commits into
Conversation
|
📲 Install BuildsAndroid
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9e60aca | 316.18 ms | 345.04 ms | 28.86 ms |
| 6b019b7 | 403.90 ms | 546.09 ms | 142.19 ms |
| d15471f | 310.66 ms | 368.19 ms | 57.53 ms |
| d217708 | 375.27 ms | 415.68 ms | 40.41 ms |
| 22f4345 | 314.79 ms | 375.02 ms | 60.23 ms |
| fcec2f2 | 328.91 ms | 387.75 ms | 58.84 ms |
| d501a7e | 307.33 ms | 341.94 ms | 34.61 ms |
| 7414e9b | 315.69 ms | 367.66 ms | 51.97 ms |
| fcec2f2 | 314.96 ms | 373.66 ms | 58.70 ms |
| e2dce0b | 308.96 ms | 360.10 ms | 51.14 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9e60aca | 0 B | 0 B | 0 B |
| 6b019b7 | 0 B | 0 B | 0 B |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| d217708 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| 22f4345 | 1.58 MiB | 2.29 MiB | 719.83 KiB |
| fcec2f2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| d501a7e | 0 B | 0 B | 0 B |
| 7414e9b | 0 B | 0 B | 0 B |
| fcec2f2 | 1.58 MiB | 2.12 MiB | 551.50 KiB |
| e2dce0b | 0 B | 0 B | 0 B |
Previous results on branch: feat/feedback-screenshot-attachment
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 37c6869 | 327.68 ms | 381.06 ms | 53.38 ms |
| 86e5409 | 326.20 ms | 351.31 ms | 25.11 ms |
| 676848b | 367.87 ms | 443.02 ms | 75.15 ms |
| 71243e7 | 313.69 ms | 370.22 ms | 56.54 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 37c6869 | 0 B | 0 B | 0 B |
| 86e5409 | 0 B | 0 B | 0 B |
| 676848b | 0 B | 0 B | 0 B |
| 71243e7 | 0 B | 0 B | 0 B |
…entry/sentry-java into feat/feedback-screenshot-attachment
runningcode
left a comment
There was a problem hiding this comment.
Thanks for adding this!
Use term "screenshot" everywhere Improve nullability handling
…hment paths Routes the three catch-all blocks around the screenshot picker through ExceptionUtils.rethrowIfFatal, so OOM, ThreadDeath and linkage errors propagate instead of being hidden, and logs the previously silent failure in getUriSize. Also moves the androidx.activity availability check out of SentryFeedbackScreenshotPicker, so the class is only loaded once the check passed, as its javadoc requires. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… buttons The form was not scrollable, so in landscape or at large font scales the send and cancel buttons were pushed off screen and the dialog could not be dismissed. Wraps it in a ScrollView. Also replaces the three full width buttons with the framework dialog button bar: cancel and send sit in a right aligned bar, and the screenshot button becomes a borderless full width action. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…istration ActivityResultRegistry.registerKey returns early for a key that is already registered, so re-registering would not have thrown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hot-attachment # Conflicts: # CHANGELOG.md # sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java # sentry-android-core/src/test/java/io/sentry/android/core/SentryUserFeedbackFormTest.kt # sentry/api/sentry.api # sentry/src/main/java/io/sentry/SentryFeedbackOptions.java # sentry/src/main/java/io/sentry/SentryOptions.java # sentry/src/test/java/io/sentry/SentryFeedbackOptionsTest.kt
…ed types AGENTS.md now forbids new catch (Throwable). Replaces the three broad catches with the types the guarded code can actually throw: - launch() documents ActivityNotFoundException and the registry throws IllegalStateException for an unregistered launcher - the ContentResolver calls cross into the provider process, where Binder can only deliver the exceptions Parcel marshals, all RuntimeException Fatal throwables now propagate because they are never caught, so ExceptionUtils.rethrowIfFatal is no longer needed here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5edad4a. Configure here.
| callback.onScreenshotPicked(uri); | ||
| } | ||
| }); | ||
| return new SentryFeedbackScreenshotPicker(launcher); |
There was a problem hiding this comment.
Shared picker key breaks stacked forms
Medium Severity
The screenshot picker always registers on the host ActivityResultRegistry under the fixed key sentry_user_feedback_screenshot_picker. A second SentryUserFeedbackForm on the same ComponentActivity overwrites that registration. Dismissing the first form then calls unregister() on the shared key and tears down the still-visible form's launcher, so its add-screenshot action fails.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5edad4a. Configure here.
androidx.activity is a compileOnly dependency, so the version in the app's apk is outside our control and may be missing the photo picker APIs this was compiled against. Catches LinkageError locally at the two call sites that link against it, then falls back to the sanctioned ExceptionUtils.rethrowIfFatal pattern, as ExceptionUtils documents and SentrySQLiteDriver.hasConnectionPool demonstrates. Registration is now guarded too, so a version mismatch hides the screenshot button instead of breaking the whole form. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
|
||
| final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions(); | ||
| maybeRegisterScreenshotPicker(options); |
There was a problem hiding this comment.
Bug: The screenshot button remains visible after being disabled. If isEnableAttachScreenshot is set to false between form displays, the button's visibility is not updated, leaving it incorrectly visible.
Severity: LOW
Suggested Fix
In the maybeRegisterScreenshotPicker() method, ensure the button's visibility is always set regardless of the isEnableAttachScreenshot() value. When disabled, explicitly set btnAddScreenshot.setVisibility(View.GONE) before the early return to correctly reflect the current configuration.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java#L415-L417
Potential issue: The visibility of the 'attach screenshot' button is not correctly
updated if the `SentryFeedbackOptions.isEnableAttachScreenshot()` setting is changed
while the feedback form is not visible. If the form is shown with screenshots enabled,
then hidden, and then the setting is disabled, the button will incorrectly remain
visible when the form is shown again. This happens because the
`maybeRegisterScreenshotPicker()` method has an early return when the feature is
disabled, which skips the logic that would hide the button. The button's visibility
state from the previous showing persists.
|
|
||
| final @NotNull SentryOptions options = Sentry.getCurrentScopes().getOptions(); | ||
| maybeRegisterScreenshotPicker(options); |
There was a problem hiding this comment.
Bug: The screenshot button remains visible after being disabled. If isEnableAttachScreenshot is set to false between form displays, the button's visibility is not updated, leaving it incorrectly visible.
Severity: LOW
Suggested Fix
In the maybeRegisterScreenshotPicker() method, ensure the button's visibility is always set regardless of the isEnableAttachScreenshot() value. When disabled, explicitly set btnAddScreenshot.setVisibility(View.GONE) before the early return to correctly reflect the current configuration.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
sentry-android-core/src/main/java/io/sentry/android/core/SentryUserFeedbackForm.java#L415-L417
Potential issue: The visibility of the 'attach screenshot' button is not correctly
updated if the `SentryFeedbackOptions.isEnableAttachScreenshot()` setting is changed
while the feedback form is not visible. If the form is shown with screenshots enabled,
then hidden, and then the setting is disabled, the button will incorrectly remain
visible when the form is shown again. This happens because the
`maybeRegisterScreenshotPicker()` method has an early return when the feature is
disabled, which skips the logic that would hide the button. The button's visibility
state from the previous showing persists.


📜 Description
Adds an "Add a screenshot" button to the Android user feedback form that opens the
androidx photo picker (
ActivityResultContracts.PickVisualMedia, images only) and sendsthe picked image as an attachment in the same envelope as the feedback.
Screen_recording_20260813_202418.mp4
SentryFeedbackOptions:enableAttachScreenshot(defaulttrue, matching the JS SDK),addScreenshotButtonLabel,removeScreenshotButtonLabel,screenshotTooLargeMessageText, plus theio.sentry.feedback.enable-attach-screenshotmanifest flagandroidx.activity:activity:1.8.2is added as compileOnly — no new transitivedependency; the button is hidden (with a warning log) when androidx.activity is missing
or the host Activity is not a
ComponentActivitymaxAttachmentSizeat pick timeFixed along the way
The form was not scrollable, so in landscape or at large font scales the send and cancel
buttons were pushed off screen and the dialog could not be dismissed. It is now wrapped in
a
ScrollView, and the three full width buttons were replaced with the framework dialogbutton bar (cancel and send in a right aligned bar, screenshot as a borderless full width
action). No changelog entry, as it is bundled with this feature.
💡 Motivation and Context
💚 How did you test it?
Unit tests + Manual testing.
📝 Checklist
sendDefaultPIIis enabled.